-
Notifications
You must be signed in to change notification settings - Fork 520
add build.ps1 to align with PSES and other PowerShell projects #1199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
build.ps1
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no new line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No copyright header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added. Wasn't sure which should be on top. Let me know if I have it wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My testing indicates that the shebang has to be on the first line of the script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, shebang must be first line. This would be the exception to the copyright header rule being first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, shebang must be first line. This would be the exception to the copyright header rule being first.
build.ps1
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just rely on $IsWindows? If it's $null, then we can assume it's Windows since that would be returned by Windows PowerShell. Same for $IsLinux and $IsMacOS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just realized OS isn't used in this script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at your logic below on lines 101-124, it seems that -Bootstrap should be it's own ParameterSet as it's mutually exclusive to -Clean and -Test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it
@tylerl0706 I think something went wrong with a push or something. Shouldn't there only be 1 file changed in this PR?
😮
8d414a4 to
e36b794
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Resolves: #1127
Here's the build.ps1 PR for PowerShellEditorServices:
PowerShell/PowerShellEditorServices#623
In scope:
build.ps1checks for missing tools and runsInvoke-Build Buildbuild.ps1 -Cleanchecks for missing tools and runsInvoke-Build Cleanand thenInvoke-Build Buildbuild.ps1 -Testchecks for missing tools and runsInvoke-Build Buildand thenInvoke-Build Testbuild.ps1 -Bootstrapinforms you of what tools you are missing